fix(editor): Restrict `[empty]` in parameter input hint to zero-length string #6003

ivov · 2023-04-18T13:41:07Z

https://linear.app/n8n/issue/ADO-529/bug-expression-rendered-result-incorrectly-shows-[empty]-when-string

https://community.n8n.io/t/if-node-testing-for-empty-field-returns-that-field-is-not-empty/25154

github-actions · 2023-04-18T13:41:37Z

codecov · 2023-04-18T13:57:08Z

Codecov Report

Patch coverage has no change and project coverage change: +0.78 🎉

Comparison is base (9c94050) 18.63% compared to head (9bfc901) 19.42%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6003      +/-   ##
==========================================
+ Coverage   18.63%   19.42%   +0.78%     
==========================================
  Files        2585     2591       +6     
  Lines      116483   117121     +638     
  Branches    18181    18389     +208     
==========================================
+ Hits        21711    22750    +1039     
+ Misses      94134    93706     -428     
- Partials      638      665      +27

see 142 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

packages/editor-ui/src/components/ParameterInputWrapper.vue

packages/editor-ui/src/components/ParameterInputHint.vue

github-actions · 2023-04-20T14:30:20Z

✅ All Cypress E2E specs passed

cypress/e2e/26-parameter-input.hint.cy.ts

packages/editor-ui/src/components/ParameterInputWrapper.vue

Pinia stores setup process WIP.

ivov · 2023-05-02T07:49:30Z

@mutdmour Unit tests for expression resolution here require a non-trivial setup, and I worry that the ratio of user value vs time spent on this PR is already way too low. Can we reduce this to the fix and revisit unit tests as tech debt?

mutdmour · 2023-05-02T08:00:24Z

@mutdmour Unit tests for expression resolution here require a non-trivial setup, and I worry that the ratio of user value vs time spent on this PR is already way too low. Can we reduce this to the fix and revisit unit tests as tech debt?

alright then let's leave it

ivov · 2023-05-02T08:26:46Z

@mutdmour Can you please re-review? Retested at da9e85f.

Workflow

{
  "name": "ADO-529",
  "nodes": [
    {
      "parameters": {},
      "id": "45de23e5-1261-4020-b949-4b58ca367b5a",
      "name": "When clicking \"Execute Workflow\"",
      "type": "n8n-nodes-base.manualTrigger",
      "typeVersion": 1,
      "position": [
        340,
        300
      ]
    },
    {
      "parameters": {
        "conditions": {
          "string": [
            {
              "value1": "={{ $json.empty_string }}",
              "operation": "isEmpty"
            },
            {
              "value1": "={{ $json.one_whitespace }}",
              "operation": "isEmpty"
            },
            {
              "value1": "={{ $json.four_whitespaces }}",
              "operation": "isEmpty"
            },
            {
              "value1": "={{ $json.normal }}",
              "operation": "isEmpty"
            }
          ]
        }
      },
      "id": "7be87064-3dc7-4a4b-8f10-246b597f2d30",
      "name": "IF",
      "type": "n8n-nodes-base.if",
      "typeVersion": 1,
      "position": [
        620,
        300
      ]
    }
  ],
  "pinData": {
    "When clicking \"Execute Workflow\"": [
      {
        "json": {
          "empty_string": "",
          "one_whitespace": " ",
          "four_whitespaces": "    ",
          "normal": "hello"
        }
      }
    ]
  },
  "connections": {
    "When clicking \"Execute Workflow\"": {
      "main": [
        [
          {
            "node": "IF",
            "type": "main",
            "index": 0
          }
        ]
      ]
    }
  },
  "active": false,
  "settings": {},
  "versionId": "1352f44e-c5ca-46b9-b275-44336d6b9c89",
  "id": "20",
  "meta": {
    "instanceId": "406f1bca875c48c0fa12bf65a32e67f001617a6df6d6fd6dd72bff9d20014812"
  },
  "tags": []
}

github-actions · 2023-05-02T08:39:07Z

⚠️ Some Cypress E2E specs are failing, please fix them before merging

mutdmour · 2023-05-02T09:04:14Z

cypress/e2e/14-mapping.cy.ts

@@ -205,7 +205,7 @@ describe('Data mapping', () => {
 				'have.text',
 				`{{ $node['${SCHEDULE_TRIGGER_NODE_NAME}'].json.input[0].count }} {{ $node['${SCHEDULE_TRIGGER_NODE_NAME}'].json.input }}`,
 			);
-		ndv.getters.parameterExpressionPreview('value').should('include.text', '[empty]');
+		ndv.getters.parameterExpressionPreview('value').should('include.text', ' ');


maybe have.text would be better here for this test

github-actions · 2023-05-02T09:15:32Z

✅ All Cypress E2E specs passed

github-actions · 2023-05-03T07:34:33Z

✅ All Cypress E2E specs passed

janober · 2023-05-03T13:50:37Z

Got released with n8n@0.227.0

* master: feat(JotForm Trigger Node): Add support for hipaa-api.jotform.com (#6171) 🚀 Release 0.227.0 (#6167) fix(editor): Flag issues only on workflow activation (#6127) fix(editor): Faster reconnects for push (no-changelog) (#6164) fix: Log unhandled errors during license activation (no-changelog) (#6165) test: Address flaky setup e2e (no-changelog) (#6085) fix(core): Better errors for common status codes fix fix(core): Fix bug running addUserActivatedColumn migration on MariaDB (#6157) fix: Prevent invocations of 'GET /rest/license' from returning an error when ephemeral licenses are used (#6154) fix(editor): Restrict `[empty]` in parameter input hint to zero-length string (#6003) fix(core): Assign Unknown Error only if message or description not present in error fix(AWS S3 Node): Fix File upload, and add node tests (#6153) fix(core): Better error message in Webhook node when using the POST method feat(Microsoft Excel 365 Node): Overhaul fix(core): Remove SAML config metadataUrl if XML metadata is set directly (#6143) ci: Restore load options methods validation (no-changelog) (#6148) feat(core): Add notice to alert users a new version is available fix(core): Fix canceled execution status (#6142) ci: Expand ESLint to tests in BE packages (no-changelog) (#6147) fix(editor): Fix focus jumping when using chrome autofill (#6140)

…rce-mapper-ui-P2 * feature/resource-mapping-component: feat(JotForm Trigger Node): Add support for hipaa-api.jotform.com (#6171) 🚀 Release 0.227.0 (#6167) fix(editor): Flag issues only on workflow activation (#6127) fix(editor): Faster reconnects for push (no-changelog) (#6164) fix: Log unhandled errors during license activation (no-changelog) (#6165) test: Address flaky setup e2e (no-changelog) (#6085) fix(core): Better errors for common status codes fix fix(core): Fix bug running addUserActivatedColumn migration on MariaDB (#6157) fix: Prevent invocations of 'GET /rest/license' from returning an error when ephemeral licenses are used (#6154) fix(editor): Restrict `[empty]` in parameter input hint to zero-length string (#6003) fix(core): Assign Unknown Error only if message or description not present in error fix(AWS S3 Node): Fix File upload, and add node tests (#6153) fix(core): Better error message in Webhook node when using the POST method feat(Microsoft Excel 365 Node): Overhaul fix(core): Remove SAML config metadataUrl if XML metadata is set directly (#6143) ci: Restore load options methods validation (no-changelog) (#6148) feat(core): Add notice to alert users a new version is available fix(core): Fix canceled execution status (#6142) ci: Expand ESLint to tests in BE packages (no-changelog) (#6147) fix(editor): Fix focus jumping when using chrome autofill (#6140)

🚧 Quick test

4cd8719

n8n-assistant bot added n8n team Authored by the n8n team ui Enhancement in /editor-ui or /design-system labels Apr 18, 2023

ivov added 2 commits April 18, 2023 16:37

⚡ Adjust to new requirements

2848917

⚡ Account for 2x double quotes

05ffa7f

ivov changed the title ~~fix(editor): Display whitespace-only strings in input hint~~ fix(editor): Quote string values in parameter input hint Apr 18, 2023

ivov marked this pull request as ready for review April 18, 2023 14:50

ivov added 3 commits April 18, 2023 17:13

🔥 Remove excess single quote

770437b

⏪ Revert per new requirements

34428fc

⏪ Undo more changes to minimize diff

b3dff4d

ivov changed the title ~~fix(editor): Quote string values in parameter input hint~~ fix(editor): Restrict [empty] in parameter input hint to zero-length string Apr 20, 2023

ivov added 2 commits April 20, 2023 12:33

⏪ New requirement changes

ed8a166

🐛 Prevent excess whitespace from HTML formatting

c535807

mutdmour reviewed Apr 20, 2023

View reviewed changes

packages/editor-ui/src/components/ParameterInputWrapper.vue Show resolved Hide resolved

packages/editor-ui/src/components/ParameterInputHint.vue Outdated Show resolved Hide resolved

ivov added 3 commits April 24, 2023 10:01

🔀 Merge master

76f961c

🚚 Rename computed property

34f6ed5

🧪 Set up e2e tests for parameter input hint

4627321

ivov requested a review from mutdmour April 24, 2023 08:51

mutdmour reviewed Apr 25, 2023

View reviewed changes

cypress/e2e/26-parameter-input.hint.cy.ts Outdated Show resolved Hide resolved

cypress/e2e/26-parameter-input.hint.cy.ts Outdated Show resolved Hide resolved

packages/editor-ui/src/components/ParameterInputWrapper.vue Outdated Show resolved Hide resolved

🚧 Experiments with unit tests

86eca35

Pinia stores setup process WIP.

ivov added 5 commits May 2, 2023 10:13

🔥 Remove e2e test

067086e

⏪ Restore test command

bba95e4

🔥 Remove unit test stub

57ad6c8

🔥 Remove logging

04fa796

🔥 Remove test ID

da9e85f

mutdmour previously approved these changes May 2, 2023

View reviewed changes

🧪 Fix e2e test

b8873ad

ivov dismissed mutdmour’s stale review via b8873ad May 2, 2023 08:47

mutdmour self-requested a review May 2, 2023 09:03

mutdmour previously approved these changes May 2, 2023

View reviewed changes

mutdmour reviewed May 2, 2023

View reviewed changes

🧪 Tighten text check

9bfc901

ivov dismissed mutdmour’s stale review via 9bfc901 May 2, 2023 09:07

ivov requested a review from mutdmour May 3, 2023 07:18

mutdmour approved these changes May 3, 2023

View reviewed changes

ivov merged commit 8862e1e into master May 3, 2023

ivov deleted the ado-529-bug-expression-rendered-result branch May 3, 2023 07:41

janober added the Released label May 3, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(editor): Restrict `[empty]` in parameter input hint to zero-length string #6003

fix(editor): Restrict `[empty]` in parameter input hint to zero-length string #6003

ivov commented Apr 18, 2023 •

edited

Loading

github-actions bot commented Apr 18, 2023 •

edited

Loading

codecov bot commented Apr 18, 2023 •

edited

Loading

github-actions bot commented Apr 20, 2023

ivov commented May 2, 2023

mutdmour commented May 2, 2023

ivov commented May 2, 2023

github-actions bot commented May 2, 2023

mutdmour May 2, 2023

github-actions bot commented May 2, 2023

github-actions bot commented May 3, 2023

janober commented May 3, 2023

fix(editor): Restrict [empty] in parameter input hint to zero-length string #6003

fix(editor): Restrict [empty] in parameter input hint to zero-length string #6003

Conversation

ivov commented Apr 18, 2023 • edited Loading

github-actions bot commented Apr 18, 2023 • edited Loading

codecov bot commented Apr 18, 2023 • edited Loading

Codecov Report

github-actions bot commented Apr 20, 2023

ivov commented May 2, 2023

mutdmour commented May 2, 2023

ivov commented May 2, 2023

github-actions bot commented May 2, 2023

mutdmour May 2, 2023

Choose a reason for hiding this comment

github-actions bot commented May 2, 2023

github-actions bot commented May 3, 2023

janober commented May 3, 2023

fix(editor): Restrict `[empty]` in parameter input hint to zero-length string #6003

fix(editor): Restrict `[empty]` in parameter input hint to zero-length string #6003

ivov commented Apr 18, 2023 •

edited

Loading

github-actions bot commented Apr 18, 2023 •

edited

Loading

codecov bot commented Apr 18, 2023 •

edited

Loading